Skip to content

ConsumeWorker: improve bank check/switch efficiency#8291

Merged
apfitzge merged 3 commits intoanza-xyz:masterfrom
apfitzge:fix_braindead_bank_check
Oct 2, 2025
Merged

ConsumeWorker: improve bank check/switch efficiency#8291
apfitzge merged 3 commits intoanza-xyz:masterfrom
apfitzge:fix_braindead_bank_check

Conversation

@apfitzge
Copy link
Copy Markdown

@apfitzge apfitzge commented Oct 1, 2025

Problem

Summary of Changes

  • Fix logic in switching/waiting for new banks
  • Only Arc::clone on actual bank switch.

Fixes #

@apfitzge apfitzge added the v3.0 label Oct 1, 2025
@mergify
Copy link
Copy Markdown

mergify Bot commented Oct 1, 2025

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 1, 2025

Codecov Report

❌ Patch coverage is 60.86957% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (82aad7b) to head (1cd80a9).
⚠️ Report is 24 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #8291     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         841      836      -5     
  Lines      368895   366658   -2237     
=========================================
- Hits       307201   305249   -1952     
+ Misses      61694    61409    -285     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@apfitzge apfitzge requested a review from alessandrod October 1, 2025 16:43
alessandrod
alessandrod previously approved these changes Oct 2, 2025
Copy link
Copy Markdown

@alessandrod alessandrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good, just nit on a method name

self.shared_working_bank.load()
/// Update the bank if it has changed.
/// Returns true if the bank is updated or still usable.
fn update_bank(&self, bank: &mut Arc<Bank>) -> bool {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: when I started reviewing I found this name confusing, since in most of the
calls it returns the current bank it doesn't update? maybe call it
update_working_bank_if_needed or something?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apfitzge apfitzge merged commit a58c605 into anza-xyz:master Oct 2, 2025
43 checks passed
mergify Bot pushed a commit that referenced this pull request Oct 2, 2025
(cherry picked from commit a58c605)

# Conflicts:
#	core/src/replay_stage.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants